-
Notifications
You must be signed in to change notification settings - Fork 18
introduce a new integrated "codeflash optimize" command #384
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
PR Reviewer Guide 🔍(Review updated until commit 9addd95)Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Latest suggestions up to 9addd95
Previous suggestionsSuggestions up to commit 4debe7e
|
08464c4
to
0b4fcb6
Compare
…(`trace-and-optimize`) Here is an optimized rewrite of your `FunctionRanker` class. **Key speed optimizations applied:** 1. **Avoid repeated loading of function stats:** The original code reloads function stats for each function during ranking (`get_function_ttx_score()` is called per function and loads/returns). We prefetch stats once in `rank_functions()` and reuse them for all lookups. 2. **Inline and batch lookups:** We use a helper to batch compute scores directly via a pre-fetched `stats` dict. This removes per-call overhead from attribute access and creation of possible keys inside the hot loop. 3. **Minimal string operations:** We precompute the two possible key formats needed for lookup (file:qualified and file:function) for all items only ONCE, instead of per invocation. 4. **Skip list-comprehension in favor of tuple-unpacking:** Use generator expressions for lower overhead when building output. 5. **Fast path with `dict.get()` lookup:** Avoid redundant `if key in dict` by just trying `dict.get(key)`. 6. **Do not change signatures or behavior. Do not rename any classes or functions. All logging, ordering, functionality is preserved.** **Summary of performance impact:** - The stats are loaded only once, not per function. - String concatenations for keys are only performed twice per function (and not redundantly in both `rank_functions` and `get_function_ttx_score`). - All lookup and sorting logic remains as in the original so results will match, but runtime (especially for large lists) will be significantly better. - If you want, you could further optimize by memoizing scores with LRU cache, but with this design, dictionary operations are already the bottleneck, and this is the lowest-overhead idiomatic Python approach. - No imports, function names, or signatures are changed. Let me know if you need further GPU-based or numpy/pandas-style speedups!
⚡️ Codeflash found optimizations for this PR📄 13% (0.13x) speedup for
|
…384 (`trace-and-optimize`) Here is an **optimized** version of your code, focusing on the `_get_function_stats` function—the proven performance bottleneck per your line profiing. ### Optimizations Applied 1. **Avoid Building Unneeded Lists**: - Creating `possible_keys` as a list incurs per-call overhead. - Instead, directly check both keys in sequence, avoiding the list entirely. 2. **Short-circuit Early Return**: - Check for the first key (`qualified_name`) and return immediately if found (no need to compute or check the second unless necessary). 3. **String Formatting Optimization**: - Use f-strings directly in the condition rather than storing/interpolating beforehand. 4. **Comment Retention**: - All existing and relevant comments are preserved, though your original snippet has no in-method comments. --- --- ### Rationale - **No lists** or unneeded temporary objects are constructed. - Uses `.get`, which is faster than `in` + lookup. - Returns immediately upon match. --- **This change will reduce total runtime and memory usage significantly in codebases with many calls to `_get_function_stats`.** Function signatures and return values are unchanged.
⚡️ Codeflash found optimizations for this PR📄 51% (0.51x) speedup for
|
…25-07-01T22.08.43 ⚡️ Speed up method `FunctionRanker._get_function_stats` by 51% in PR #384 (`trace-and-optimize`)
This PR is now faster! 🚀 @misrasaurabh1 accepted my optimizations from: |
Persistent review updated to latest commit 9addd95 |
…imize`) Here's an optimized version of your Python program, focused on runtime and memory. **Key changes:** - Avoids reading the event file or parsing JSON if not needed. - Reads the file as binary and parses with `json.loads()` for slightly faster IO. - References the `"draft"` property directly using `.get()` to avoid possible `KeyError`. - Reduces scope of data loaded from JSON for less memory usage. - Caches the result of parsing the event file for repeated calls within the same process. - The inner try/except is kept close to only catching the specific case. - Results for each event_path file are cached in memory. - Exception handling and comments are preserved where their context is changed. - I/O and JSON parsing is only done if both env vars are set and PR number exists.
⚡️ Codeflash found optimizations for this PR📄 121% (1.21x) speedup for
|
… trace-and-optimize
@misrasaurabh1 ready to review, can't tag you normally b/c you're the author |
code_to_optimize/code_directories/simple_tracer_e2e/codeflash.trace
Outdated
Show resolved
Hide resolved
pyproject.toml
Outdated
[tool.pytest.ini_options] | ||
filterwarnings = [ | ||
"ignore::pytest.PytestCollectionWarning", | ||
"ignore::pytest.PytestUnknownMarkWarning" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reasoning for PytestUnknownMarkWarning
@KRRT7 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any examples of PytestUnknownMarkWarning
specifically you've seen so far. I've not seen them yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it gets triggered on the pytests that have the skip_ci markers
PR Type
Enhancement, Tests
Description
Introduce integrated
optimize
CLI commandAdd
FunctionRanker
class for ttX-based rankingExtend tracer: generate replay test and invoke optimizer
Update tests: unit and end-to-end optimize flows
Changes diagram
Changes walkthrough 📝
7 files
Add sleep and heavy compute in SimpleModel.predict
New `FunctionRanker` for function profiling ranking
Add `optimize` subcommand and CI flag parsing
Add `is_ci()` helper for CI environment detection
Integrate trace path and rerank functions workflow
Support replay tests, static methods, optimization chaining
Include `class_name` and normalize caller keys
2 files
Define `DEFAULT_IMPORTANCE_THRESHOLD` constant
Configure pytest warning filters
3 files
Fix import order and comma formatting in LSP server
Change `setup_logging` return type signature
Clean up trailing comments and commas
1 files
Remove debug print in placeholder creation
3 files
Update traced count and coverage expectations
Switch to `codeflash.main optimize` invocation
Add unit tests for `FunctionRanker` methods
1 files